Repository navigation
feat: Add request body compression with optional brotli - #927
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #927 +/- ##
==========================================
+ Coverage 94.54% 94.59% +0.04%
==========================================
Files 48 58 +10
Lines 5100 5239 +139
==========================================
+ Hits 4822 4956 +134
- Misses 278 283 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
49bf689 to
aabd87c
Compare
- apify/apify-core#28971 added support for brotli compression to BE. - Pros: higher compression, cons: more CPU intensive. - The brotli dependencies are defined as optional, one has to explicitly enable it and choose one. If none is available, the code falls back to gzip. - JS client doesn't compress requests that are too small, this Python client compresses just everything. No change done, I only noticed and stating it.
aabd87c to
f26780a
Compare
|
This has a significant impact. @Pijukatel, please check it as well. |
- Remove `brotlicffi`, only CPython runtime is supported. - Add a test with bytearray as an input. This would break only in `brotlicffi`, which is no longer in the dependencies. `brotli` dependency handles it correctly and doesn't throw. - Add `brotli` to dev dependencies and use it directly in tests.
- apify/apify-client-python#927 (comment) - The default was 11 (max). Now it uses quality 6 - roughly 2–4× faster with only a modest increase in compressed size.
|
@vdusek Nearly all comments are fixed now, thanks again for your great review. One more thing: I forgot to mention that the JS client skips compression if the payload size is below 1024 B. Do we want to apply it to the Python client as well? |
- apify/apify-client-python#927 (comment) - The default was 11 (max). Now it uses quality 6 - roughly 2–4× faster with only a modest increase in compressed size.
- apify/apify-client-python#927 (comment) - The default was 11 (max). Now it uses quality 6 - roughly 2–4× faster with only a modest increase in compressed size.
Thanks for noticing. I opened an issue for that: #934. Let's resolve it in a separate PR so it is documented in the changelog. |
Pijukatel
left a comment
There was a problem hiding this comment.
Not sure about the non-default compression quality , but it is not blocking for me.
vdusek
left a comment
There was a problem hiding this comment.
I removed brotli from the dev dependency group. Sorry, my previous comment wasn't entirely correct. We run uv sync --all-extras everywhere in CI, so there's no need to include it there. The initial mypy/ty override ignores confused me, but they were probably added based on your local setup, where you didn't have the brotli extra installed.
Other than that, one more thing: the documentation 🙂.
Please extend the installation section in the docs here: https://docs.apify.com/api/client/python/docs#installation. You can use a similar section to the one you added to the README.
Also, please create a new concept page for compression. Something like: explaining HTTP compression, how to switch between compression methods, and the pros and cons of each approach.
- apify/apify-core#28971 added support for brotli compression to BE. - Pros: higher compression, cons: more CPU intensive. - The code could be running on too old Node.js, so brotli compression is applied only if possible; the code otherwise falls back to gzip. Too small payloads and unsupported types are still not compressed - no change. ### Related PRs - apify/apify-core#28971 - apify/apify-docs#2750 - #962 - apify/apify-client-python#927 - apify/apify-sdk-python#1031
vdusek
left a comment
There was a problem hiding this comment.
I did a few additional improvements. @Pijukatel do you want to re-check it?
### TLDR - Ports the `http_compressors` design of the Python client (apify/apify-client-python#927) to the JS client. - Closes: #1064 ### Description Request compression was hard-wired in `maybeCompressValue()`: brotli at quality 6, a silent fallback to gzip, and no way to pick the algorithm, tune the quality, or plug in a custom compressor. The client now has an abstract `HttpCompressor` (`contentEncoding` plus an async `compress()`), a `GzipHttpCompressor` (quality 1 to 9, default 9) and a `BrotliHttpCompressor` (quality 0 to 11, default 6), all exported from the package. `ApifyClient` takes a `compression` option: `'brotli'` (the default), `'gzip'`, or an `HttpCompressor` instance. The option is validated by shape, so an instance from a second copy of the package passes too. `HttpClient` holds the resolved compressor and the compress interceptor uses it. The rules for which bodies get compressed are unchanged: at least 1 KiB, a compressible content type, no caller-set `Content-Encoding`, Node.js only. Two deliberate differences from Python. Brotli stays the default, since Node ships it in `node:zlib` and the dependency reason for Python's gzip default doesn't apply. Streams are never compressed, where Python reads file-likes into memory. `setRecord()` still has no `contentEncoding` option, so the docs route pre-compressed bodies through `client.httpClient.call()`. Docs: a new `HTTP compression` concepts page adapted from the Python one, a section in the v3 upgrade guide, and the regenerated api-extractor report. ### Breaking change The silent fallback from #990 is gone. The configured compressor is the only one the client runs, and its error fails the request, as in Python. A runtime whose `node:zlib` lacks brotli used to get a gzipped or uncompressed body and now needs `compression: 'gzip'`. A client constructed without the option sends the same requests as before. *✍️ Drafted by Claude Code*
### TLDR - Ports the `http_compressors` design of the Python client (apify/apify-client-python#927) to the JS client. - Closes: #1064 ### Description Request compression was hard-wired in `maybeCompressValue()`: brotli at quality 6, a silent fallback to gzip, and no way to pick the algorithm, tune the quality, or plug in a custom compressor. The client now has an abstract `HttpCompressor` (`contentEncoding` plus an async `compress()`), a `GzipHttpCompressor` (quality 1 to 9, default 9) and a `BrotliHttpCompressor` (quality 0 to 11, default 6), all exported from the package. `ApifyClient` takes a `compression` option: `'brotli'` (the default), `'gzip'`, or an `HttpCompressor` instance. The option is validated by shape, so an instance from a second copy of the package passes too. `HttpClient` holds the resolved compressor and the compress interceptor uses it. The rules for which bodies get compressed are unchanged: at least 1 KiB, a compressible content type, no caller-set `Content-Encoding`, Node.js only. Two deliberate differences from Python. Brotli stays the default, since Node ships it in `node:zlib` and the dependency reason for Python's gzip default doesn't apply. Streams are never compressed, where Python reads file-likes into memory. `setRecord()` still has no `contentEncoding` option, so the docs route pre-compressed bodies through `client.httpClient.call()`. Docs: a new `HTTP compression` concepts page adapted from the Python one, a section in the v3 upgrade guide, and the regenerated api-extractor report. ### Breaking change The silent fallback from #990 is gone. The configured compressor is the only one the client runs, and its error fails the request, as in Python. A runtime whose `node:zlib` lacks brotli used to get a gzipped or uncompressed body and now needs `compression: 'gzip'`. A client constructed without the option sends the same requests as before. *✍️ Drafted by Claude Code*
Closes: #942
Related PRs